internal/service/user_test.go (1)

302-303: Consider using a more deterministic approach for async audit log verification.

The time.Sleep(500 * time.Millisecond) introduces potential flakiness in CI environments under load. However, this is a common pragmatic pattern for testing async behavior when the audit service doesn't expose a synchronization mechanism.

If flakiness becomes an issue, consider polling with a short interval and timeout:

♻️ Alternative polling approach

// Poll for audit log with timeout instead of fixed sleep
deadline := time.Now().Add(2 * time.Second)
var found bool
for time.Now().Before(deadline) {
    logs, err := store.ListAuditLogs(ctx, org.ID, domain.AuditFilter{
        Action: &action,
        Limit:  10,
    })
    if err == nil {
        for _, log := range logs {
            if log.ResourceID != nil && *log.ResourceID == user.ID.String() {
                found = true
                break
            }
        }
    }
    if found {
        break
    }
    time.Sleep(50 * time.Millisecond)
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/service/user_test.go` around lines 302 - 303, Replace the fixed
time.Sleep(500 * time.Millisecond) in the test with a deterministic polling
approach: repeatedly call store.ListAuditLogs(ctx, org.ID,
domain.AuditFilter{Action: &action, Limit: 10}) on a short interval (e.g., 50ms)
until you find an audit entry whose ResourceID matches user.ID.String() or a
deadline (e.g., 2s) elapses; fail the test if the deadline is reached without
finding the expected log. This change removes the flaky fixed sleep and makes
verification robust for the async audit goroutine.

internal/service/user.go (1)

43-59: Standardize error wrapping with service/function context.

On Lines 43-59, some returns are unwrapped (return nil, err) or use non-standard prefixes ("hash password: %w", "create user: %w"). This weakens traceability and makes error handling less uniform across services.

💡 Suggested wrapping pattern

 	if err := ValidatePassword(in.Password); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("service.UserService.Create: %w", err)
 	}
 ...
 	hashed, err := bcrypt.GenerateFromPassword([]byte(in.Password), bcrypt.DefaultCost)
 	if err != nil {
-		return nil, fmt.Errorf("hash password: %w", err)
+		return nil, fmt.Errorf("service.UserService.Create: hash password: %w", err)
 	}
 ...
-		return nil, fmt.Errorf("create user: %w", err)
+		return nil, fmt.Errorf("service.UserService.Create: create user: %w", err)
 	}

As per coding guidelines "Always use sentinel errors from internal/domain/errors.go. Wrap errors with context using fmt.Errorf() in the format: fmt.Errorf(\"<package>.<function>: %w\", <error>)."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/service/user.go` around lines 43 - 59, Wrap all errors returned from
this block with the standardized package.function sentinel pattern instead of
returning raw errs or ad-hoc prefixes; e.g., replace bare returns like `return
nil, err` and messages like `fmt.Errorf("hash password: %w", err)` /
`fmt.Errorf("create user: %w", err)` with `fmt.Errorf("service.CreateUser: %w",
err)` (or use the exact service method name if different) and when mapping
domain unique constraint use `fmt.Errorf("service.CreateUser: %w",
domain.ErrAlreadyExists)` combined with the pg error check; update the
ValidatePassword, bcrypt.GenerateFromPassword, and s.store.CreateUser error
returns to follow this `fmt.Errorf("<package>.<function>: %w", err)` convention
referencing ValidatePassword, bcrypt.GenerateFromPassword, and
s.store.CreateUser locations.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes